Skip to content

Conversation

@daltontf
Copy link
Contributor

List View - There is only one way to get to this page, the "List" button.

This addressed how this was implemented by creating a different route to the list.

List View - There is only one way to get to this page, the "List" button.

This addressed how this was implemented by creating a different route to the list.

Signed-off-by: Tim Dalton <daltont@objectcomputing.com>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was duplicate code

Signed-off-by: Tim Dalton <daltont@objectcomputing.com>
export let sideBarHidden = false;
export let mainContentHidden = false;
$: layoutContainerClass = `layout-container-${sideBarHidden}-${mainContentHidden}`;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this approach works, but it results in CSS class names that aren't very meaningful. How about something like this?

$: layoutContainerClass = listHidden ? 'map-only' : mapHidden ? : list-only' ? 'list-and-map';

This assumes that the variable sideBarHidden is renamed to listHidden and mainContentHidden is renamed to mapHidden.

The CSS class names would have to be changed to match the names used above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commited fix.

Originally had this idea of keeping the SideBarMainContentLayout as a generic resusable thing and with side-bar / main-content, but went ahead and renamed the component.

.layout-container-false-true {
display: grid;
grid-template-columns: auto;
height: 100%;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like height: 100% is always what is desired and shouldn't need to be specified for all three cases.

}
aside {
.side-bar {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this CSS class should be named list-view to make it more clear.

…ccessibility issue #66.

Signed-off-by: Tim Dalton <daltont@objectcomputing.com>
Signed-off-by: Tim Dalton <daltont@objectcomputing.com>
mvolkmann
mvolkmann previously approved these changes Jan 19, 2026
Signed-off-by: Tim Dalton <daltont@objectcomputing.com>
Signed-off-by: Tim Dalton <daltont@objectcomputing.com>
Signed-off-by: Tim Dalton <daltont@objectcomputing.com>
@daltontf daltontf merged commit c2acbe7 into main Jan 20, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants